Skip to content

fix: allow retrying partial worktree removal - #3315

Merged
JJLiebig merged 1 commit into
herdrdev:masterfrom
akbash-bot:akbash/3314-retry-worktree-removal
Sep 12, 2026
Merged

fix: allow retrying partial worktree removal#3315
JJLiebig merged 1 commit into
herdrdev:masterfrom
akbash-bot:akbash/3314-retry-worktree-removal

Conversation

@akbash-bot

@akbash-bot akbash-bot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Issue

After Git unregisters a worktree but cannot delete its directory, retrying removal in the open dialog keeps showing an error.

Problem

Git reports that the checkout is no longer a working tree. The dialog only offered force confirmation for dirty checkouts, leaving guarded recovery inaccessible from this retry state.

How did we fix it?

Herdr now offers explicit force confirmation for this removal error. The fix lives in the client-owned dialog after the TUI migration. API responses and filesystem safety checks are unchanged. Reopening a fresh dialog after the checkout disappears from Git's list remains outside this fix.

Verification

The current client interaction test failed before the fix and passes afterward. It checks the warning, explicit forced retry, no automatic retry, unrelated errors, forced errors, and cancellation. All 17 targeted removal tests and full just check pass, including 3,402 Rust tests, Windows cross-lint, maintenance, integration assets, and docs checks.

refs #3314

@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 1fd8d513-72aa-48fa-83e9-b8c8cb84cee0

📥 Commits

Reviewing files that changed from the base of the PR and between 0b02595 and d4cd005.

📒 Files selected for processing (2)
  • src/client/shell/tests/agents_worktrees_notifications.rs
  • src/client/shell/worktrees.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Worktree removal now requests force confirmation for matching worktree_remove_failed errors in addition to dirty_worktree_requires_force. The test covers dirty, fatal working-tree, permission, and server-unavailable errors. It verifies removal state, confirmation state, retry behavior, modal text, forced removal, and overlay dismissal.

Suggested reviewers: ogulcancelik

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to d4cd0

The recovery flow only permits removal after explicit confirmation and preserves the existing safety checks for leftover directories. No merge-blocking risk is identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 71.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 4 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly identifies the main change: enabling retries for partial worktree removal.
Description check ✅ Passed The description directly explains the partial worktree removal failure, the force-confirmation fix, and the verification results.
  • Fix all pre-merge checks with AI

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kangal-bot kangal-bot added the ai-review Trigger automated AI reviews for pull requests admitted by the PR gate label Aug 27, 2026
@akbash-bot
akbash-bot force-pushed the akbash/3314-retry-worktree-removal branch from 0b02595 to c70825e Compare August 27, 2026 19:39
@greptile-apps

greptile-apps Bot commented Aug 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR allows an interrupted worktree removal to be retried when Git has already unregistered the checkout but left its directory behind.

  • Recognizes Git’s “not a working tree” failure as requiring explicit force confirmation.
  • Preserves the existing API error code and original Git message.
  • Adds a regression test covering deferred API removal state, response behavior, and absence of premature workspace or event mutations.

Confidence Score: 5/5

The PR appears safe to merge because the new retry state remains explicitly confirmed and the existing forced-cleanup guards continue to reject registered or unrelated directories.

The change only enables confirmation after a recognized partial-removal error; operation identity and path matching protect modal state, and filesystem cleanup remains gated by registration and repository-identity checks.

Important Files Changed

Filename Overview
src/app/api/worktrees/deferred.rs Extends deferred removal error handling to request explicit force confirmation for recognized partial-removal errors while retaining existing response semantics and path/state matching.
src/app/api/worktrees.rs Adds focused regression coverage verifying the response, pending-operation cleanup, modal transition, and preservation of workspace and event state.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Deferred worktree removal] --> B{Git result}
  B -->|Success| C[Remove workspace and emit events]
  B -->|Dirty checkout| D[Return dirty_worktree_requires_force]
  B -->|Not a working tree| E[Return worktree_remove_failed]
  D --> F[Show explicit force confirmation]
  E --> F
  F --> G[Confirmed forced retry]
  G --> H{Path still registered or unrelated?}
  H -->|Yes| I[Reject cleanup]
  H -->|No, valid repository leftover| J[Remove leftover directory]
Loading

Reviews (1): Last reviewed commit: "fix: allow retrying partial worktree rem..." | Re-trigger Greptile

@JJLiebig

Copy link
Copy Markdown
Collaborator

@akbash-bot check if the feature is still needed, rebase and resolve conflicts, then get CI/reviews happy

@akbash-bot
akbash-bot force-pushed the akbash/3314-retry-worktree-removal branch from c70825e to d4cd005 Compare September 12, 2026 19:34
@akbash-bot

Copy link
Copy Markdown
Collaborator Author

Still needed: the regression test fails on current master when an open removal dialog receives Git’s “not a working tree” error. Rebased onto d184b41 and ported the fix to the client-owned dialog in d4cd005; server/API and deletion safeguards remain unchanged.

Full just check passes, including Windows cross-lint and 3,402 Rust tests. Linux, macOS, Windows, and packaging CI are green. CodeRabbit reviewed the new head without actionable code findings; no Greptile check appeared for this head.

The generic docstring warning counts four files, but this refreshed diff touches two and adds no function. I am leaving unrelated documentation unchanged.

Ready for maintainer review. Scope remains retrying within the open dialog; reopening after the checkout disappears from Git’s list is not addressed here.

@JJLiebig
JJLiebig self-requested a review September 12, 2026 19:54
@JJLiebig
JJLiebig merged commit 96f2324 into herdrdev:master Sep 12, 2026
6 checks passed
geril07 pushed a commit to geril07/herdr that referenced this pull request Sep 13, 2026
refs herdrdev#3314

Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Trigger automated AI reviews for pull requests admitted by the PR gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants